-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add thread-per-test-class execution model #3941
Conversation
d466d4c
to
784cfc2
Compare
The thread-per-test-class execution model runs tests via one thread per test class. This model is primarily useful in situations when test executions attach `ThreadLocal`s to the test worker thread, which "leak" into other test class executions. This problem can lead to out-of-memory errors, if the `ThreadLocal`s reference (large) object trees, either as its value or via its initial-value `Supplier`, which cannot be cleaned up / garbage collected, because the `ThreadLocal` is referenced by the test worker thread. The problem becomes even worse, if the test class creates its own class loader and attaches `ThreadLocal`s that reference classes loaded by such class loaders. In such cases the whole class loader including all its loaded classes and (static) state is not eligible for garbage collection, leaking more heap (and non-heap) memory. Using one thread per test class works around the above problem(s), because once the (ephemeral) thread per test-class finishes, the whole thread and all its `ThreadLocal`s become eligible for garbage collection. Particularly Quarkus unit tests (`@QuarkusTest` annotated test classes) benefit from this execution model. This change cannot eliminate other sources of similar leaks, like threads spawned from tests or not removed MBeans. Those kinds of leaks are better handled by the test code or the tested code providing "proper" cleanup mechanisms. This new execution is implemented via the introduced `ThreadPerClassHierarchicalTestExecutorService`, a 3rd model in addition to `SameThreadHierarchicalTestExecutorService` and `ForkJoinPoolHierarchicalTestExecutorService`. It is enabled if `junit.jupiter.execution.threadperclass.enabled` is set to `true` and `junit.jupiter.execution.parallel.enabled` is `false`. Issue: junit-team#3939
784cfc2
to
f42e100
Compare
|
/cc @sormuras |
|
||
UniqueId.Segment lastSegment = testDescriptor.getUniqueId().getLastSegment(); | ||
|
||
if ("class".equals(lastSegment.getType())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an implementation detail of the JUnit Jupiter Engine so it should not be used in the platform module which is test engine agnostic. Perhaps you can use TestDescriptor.getSource
instead to potentially obtain the class source?
Note: I'm not part of the JUnit Team, just commenting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I'm not convinced whether it's addressing the problem on the right level, though. An alternative would be for Jupiter to provide a new ClassExecutionInterceptor
extension point (or a new method in InvocationInterceptor
) that would be called from ClassTestDescriptor.around
. I'll bring this up for discussion with the team.
Thanks for your PR! Closing with rationale in #3939 (comment). |
The thread-per-test-class execution model runs tests via one thread per test class. This model is primarily useful in situations when test executions attach
ThreadLocal
s to the test worker thread, which "leak" into other test class executions. This problem can lead to out-of-memory errors, if theThreadLocal
s reference (large) object trees, either as its value or via its initial-valueSupplier
, which cannot be cleaned up / garbage collected, because theThreadLocal
is referenced by the test worker thread.The problem becomes even worse, if the test class creates its own class loader and attaches
ThreadLocal
s that reference classes loaded by such class loaders. In such cases the whole class loader including all its loaded classes and (static) state is not eligible for garbage collection, leaking more heap (and non-heap) memory.Using one thread per test class works around the above problem(s), because once the (ephemeral) thread per test-class finishes, the whole thread and all its
ThreadLocal
s become eligible for garbage collection.Particularly Quarkus unit tests (
@QuarkusTest
annotated test classes) benefit from this execution model.This change cannot eliminate other sources of similar leaks, like threads spawned from tests or not removed MBeans. Those kinds of leaks are better handled by the test code or the tested code providing "proper" cleanup mechanisms.
This new execution is implemented via the introduced
ThreadPerClassHierarchicalTestExecutorService
, a 3rd model in addition toSameThreadHierarchicalTestExecutorService
andForkJoinPoolHierarchicalTestExecutorService
. It is enabled ifjunit.jupiter.execution.threadperclass.enabled
is set totrue
andjunit.jupiter.execution.parallel.enabled
isfalse
.Resolves #3939.
Overview
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@API
annotations